OCPBUGS-60936:fix(hccp): remove immutable field from kubeletconfigmap - #6726
Conversation
|
@Tal-or: This pull request references Jira Issue OCPBUGS-60936, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThe mutateKubeletConfig function no longer sets the hosted KubeletConfig mirror ConfigMap as immutable; it still merges labels and copies data from the control-plane ConfigMap and returns the updated object. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Reconciler
participant Resources as Resources.mutateKubeletConfig
participant CPConfig as ControlPlane ConfigMap
participant HostedConfig as Hosted ConfigMap (mirrored)
Reconciler->>Resources: mutateKubeletConfig()
Resources->>CPConfig: Read labels & data
Resources->>HostedConfig: Merge labels, copy data
note right of HostedConfig #DFF2E1: Immutable flag not set (mutable)
Resources-->>Reconciler: Return updated Hosted ConfigMap
sequenceDiagram
autonumber
actor Reconciler
participant ResourcesPrev as Resources.mutateKubeletConfig (previous)
participant HostedConfigPrev as Hosted ConfigMap
Reconciler->>ResourcesPrev: mutateKubeletConfig()
ResourcesPrev->>HostedConfigPrev: Set `Immutable = true`
note right of HostedConfigPrev #FDE2E2: Previous behavior prevented future mutations
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
/jira-refresh |
|
/jira refresh |
|
@Tal-or: This pull request references Jira Issue OCPBUGS-60936, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@Tal-or: This pull request references Jira Issue OCPBUGS-60936, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Test Resultse2e-aws
e2e-aks
|
f78a7e2 to
ba90992
Compare
|
@Tal-or: This pull request references Jira Issue OCPBUGS-60936, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
|
@Tal-or: This pull request references Jira Issue OCPBUGS-60936, which is invalid:
Comment DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@Tal-or: No Jira issue is referenced in the title of this pull request. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
ba90992 to
13539c4
Compare
|
Hi @csrwng |
|
/verified by @mrniranjan |
|
@mrniranjan: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/approve |
|
/approve cancel |
When kubeletconfig is changing due to manual user changes, or due to performance profile changes, hccp should catch up and mirror the changes to the hosted cluster. When setting the kubeletconfig configmap as immutable, the controller failed to update the configmap with the following error: `ConfigMap "<config-map-name>" is invalid: data: Forbidden: field is immutable when `immutable` is set`. Setting the ConfigMap as immutable was redundant because in case of undesired change, hccp will reconcile the ConfigMap and update it to its desired state, so it's safe to remove it. NOTE: It’s still possible for the ConfigMap on the hosted cluster side to be tampered with, but accepting this small risk is a reasonable trade-off to ensure the system functions correctly. Signed-off-by: Talor Itzhak <titzhak@redhat.com>
abda02b to
bbd8f63
Compare
|
/hold cancel |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: csrwng, Tal-or The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
Scheduling tests matching the |
|
/retest |
|
/verified by @Tal-or |
|
@Tal-or: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@Tal-or: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@Tal-or: Jira Issue Verification Checks: Jira Issue OCPBUGS-60936 Jira Issue OCPBUGS-60936 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
What this PR does / why we need it:
When kubeletconfig is changing due to manual user changes, or due to performance profile changes, hccp should catch up and mirror the changes to the hosted cluster.
When setting the kubeletconfig configmap as immutable, the controller failed to update the configmap with the following error:
ConfigMap "<config-map-name>" is invalid: data: Forbidden: field is immutable when immutable is set.Setting the ConfigMap as immutable was redundant because in case of undesired change, hccp will reconcile the ConfigMap and update it to its desired state, so it's safe to remove it.
NOTE: It’s still possible for the ConfigMap on the hosted cluster side to be tampered with, but accepting this small risk is a reasonable trade-off to ensure the system functions correctly.
Which issue(s) this PR fixes
Fixes # OCPBUGS-60936
Checklist
Summary by CodeRabbit
New Features
Bug Fixes